fix: watermark filter use commited epoch to read global watermark #17724
Conversation
st1page
added
need-cherry-pick-release-1.10
| barrier_num_during_idle += 1; | ||
|
|
||
| if barrier_num_during_idle == UPDATE_GLIOBAL_WATERMARK_FREQUENCY_WHEN_IDLE { |
There was a problem hiding this comment.
Without it, the issue will still occur when "relace the table", as it cannot be guaranteed that all actors have been dropped by the time execution reaches here. After adding this logic, it will wait for 10 barriers before proceeding to this point, thereby avoiding the issue.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
| current_watermark = Self::get_global_max_watermark( | ||
| &table, | ||
| &global_watermark_table, | ||
| HummockReadEpoch::Committed(prev_epoch), |
There was a problem hiding this comment.
I guess NoWait is still okay here, cuz we're sure prev_epoch of the Pause checkpoint is committed here.
risingwave/src/meta/src/barrier/command.rs
Lines 892 to 900 in 4605773
By NoWait on update.prev_epoch (i.e. pause.curr_epoch), we're essentially read on the exactly same snapshot as pause.prev_epoch because we ensure that during the Pause barrier no data is written.
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
…7724) (#17762) Co-authored-by: stonepage <[email protected]>
…7724) (#17761) Co-authored-by: stonepage <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
fix #16836
But when making modifications, it was found that, in addition to initialization and scaling, querying the global watermark is also necessary when the source is idle. In this situation, using epoch to query could lead to a degradation in concurrent checkpointing, as it would require waiting for the epoch Here, reading with an specific epoc is too restrictive. In fact, what the watermark filter needs here is "to query the most recently committed version available, the newer the better" That is, `get_row(pk, HummockReadEpoch::Committed(current_max_commited_epoch))`.Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.